Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement RFC 1047 - socket timeouts #25818

Merged
merged 4 commits into from
May 30, 2015
Merged

Conversation

sfackler
Copy link
Member

Closes #25619

r? @alexcrichton

// Timeouts
////////////////////////////////////////////////////////////////////////////////

#[cfg(target_os = "windows")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move this platform-specific code into src/libstd/sys/{unix,windows}/net.rs ?

@alexcrichton
Copy link
Member

Nice! Could you also add some tests that exercise the I/O portion of timeouts? For example:

  • If a read timeout is set, it shouldn't block for longer than that
  • If a millisecond timeout is set, a read should be able to succeed if there's pending data, and if you wait for 2ms the next read should only time out after 1ms.
  • Write timeouts may be hard to test, but it would be nice to see that a hanging write also returns eventually.

@sfackler
Copy link
Member Author

Updated. The time comparisons in the new tests are pretty lax, but I didn't want them to flicker and it still makes sure it doesn't immediately return and doesn't block for way too long.

I couldn't really think of a reasonable way to test write timeouts unfortunately since the kernel will buffer writes.

@sfackler
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented May 28, 2015

⌛ Trying commit 97040aa with merge ab9d398...

bors added a commit that referenced this pull request May 28, 2015
@bors
Copy link
Contributor

bors commented May 28, 2015

💔 Test failed - try-mac

@sfackler
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented May 28, 2015

⌛ Trying commit 6768ca9 with merge ee47d85...


/// Returns the read timeout of this socket.
///
/// If the timeout is `None`, then `read` calls will block indefinitely.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these docs also mention that not all platforms support accessing the read/write timeouts, even if they have been set successfully?

@alexcrichton
Copy link
Member

r=me once tests are passing, thanks @sfackler!

@sfackler
Copy link
Member Author

@bors try

Should be good to go now, but I'll run it past try one more time.

bors added a commit that referenced this pull request May 28, 2015
@bors
Copy link
Contributor

bors commented May 28, 2015

⌛ Trying commit 8b758a3 with merge 172e860...

@sfackler
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented May 29, 2015

⌛ Trying commit 69a0e1a with merge 991249b...

@bors
Copy link
Contributor

bors commented May 29, 2015

💔 Test failed - try-linux

@sfackler
Copy link
Member Author

@bors try

bors added a commit that referenced this pull request May 29, 2015
@bors
Copy link
Contributor

bors commented May 29, 2015

⌛ Trying commit e632166 with merge 1864973...

@bors
Copy link
Contributor

bors commented May 29, 2015

💔 Test failed - try-win-gnu-64

@sfackler
Copy link
Member Author

@bors r=alexcrichton 494901a

@bors
Copy link
Contributor

bors commented May 29, 2015

⌛ Testing commit 494901a with merge d8b877c...

@bors
Copy link
Contributor

bors commented May 29, 2015

💔 Test failed - auto-linux-64-x-android-t

@sfackler
Copy link
Member Author

@bors r=alexcrichton b5c6c7e

@bors
Copy link
Contributor

bors commented May 30, 2015

⌛ Testing commit b5c6c7e with merge b9daa8f...

@bors
Copy link
Contributor

bors commented May 30, 2015

💔 Test failed - auto-linux-64-opt

@sfackler
Copy link
Member Author

@bors retry

@bors
Copy link
Contributor

bors commented May 30, 2015

⌛ Testing commit b5c6c7e with merge 474c6e0...

@bors bors merged commit b5c6c7e into rust-lang:master May 30, 2015
@sfackler sfackler deleted the socket-timeouts branch May 30, 2015 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement socket timeouts
3 participants